Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only set Albany python exec. if +py #27

Merged
merged 1 commit into from
Mar 18, 2024

Conversation

xylar
Copy link
Collaborator

@xylar xylar commented Mar 17, 2024

This fixed the issue reported here for me:
MPAS-Dev/compass#793 (comment)

This may be a change in how either CMake or spack behaves by default. Setting a field to an empty string isn't the same as not defining it.

@xylar xylar requested a review from ikalash March 17, 2024 08:51
Copy link

@ikalash ikalash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for tracking this down! I was going to look at this today but it looks like you beat me to it.

@xylar
Copy link
Collaborator Author

xylar commented Mar 18, 2024

Thanks @ikalash!

@xylar xylar merged commit f65baad into E3SM-Project:develop Mar 18, 2024
8 of 11 checks passed
@xylar xylar deleted the fix-albany-python-executable branch March 18, 2024 10:16
@kliegeois
Copy link

Thanks @xylar for that fix! Sorry for introducing this bug in the first place.

@xylar
Copy link
Collaborator Author

xylar commented Mar 18, 2024

@kliegeois, ah thanks for pointing that out! I hadn't realized the change was introduced recently but that explains a lot! No worries!

@xylar
Copy link
Collaborator Author

xylar commented Mar 18, 2024

@ikalash, how hard would it be to introduce an Albany build to your testing that uses the settings we're using in Compass (albany+mpas~py~epetra+unit_tests)? I think such a test would have caught this problem.

@ikalash
Copy link

ikalash commented Mar 18, 2024

@xylar : it would not be hard. We are currently testing the following build: albany@develop%[email protected]+mpas+py+optimization+mesh_depends_on_params+omegah. I would suggest to add ~epetra and +unit_tests to this build to avoid having multiple nightly test builds. @mperego are you OK with this?

@xylar
Copy link
Collaborator Author

xylar commented Mar 18, 2024

@ikalash, the critical piece is actually the ~py (not having the python interface). That would almost certainly have to be a separate test, since you will also test with python. If you just want one test, I think what you've got it the better option.

@xylar
Copy link
Collaborator Author

xylar commented Mar 18, 2024

The ~epetra+unit_tests options should be the defaults. I just specified them explicitly because I was getting builds with epetra for some reason and I wanted to make sure I wasn't getting passing builds only by skipping the unit tests.

@ikalash
Copy link

ikalash commented Mar 18, 2024

@xylar : I see - I misread the ~py, sorry. Yes, I could add the new build. I'd do it probably on a different machine since there is already so much testing happening on camobap. I'll let you know when the new nightly build has been setup.

@jewatkins
Copy link

I think we're only using the spack build for compass currently? In which case, it makes sense to me to test what compass is using. i.e. probably don't need a new build.

Eventually it probably makes sense to have all testing through spack builds but I think we're still far from that.

@ikalash
Copy link

ikalash commented Mar 18, 2024

I think we're only using the spack build for compass currently? In which case, it makes sense to me to test what compass is using. i.e. probably don't need a new build.

Do you mean Xylar is using compass for the spack build? Just to be clear, my nightly tests do not do anything with compass, they just build Albany.

@jewatkins
Copy link

xylar is using the albany spack build for compass. I'm not aware of anyone else using the albany spack build.

@ikalash
Copy link

ikalash commented Mar 18, 2024

xylar is using the albany spack build for compass. I'm not aware of anyone else using the albany spack build.

But the error @xylar reported was from building Albany, no?

@ikalash
Copy link

ikalash commented Mar 18, 2024

It looks like @xylar 's build line is:

albany@compass-2024-03-13%[email protected]+64bit+confgui~debug+epetra~fpe~ipo+landice~mesh_depends_on_params+mpas~omegah~optimization~perf~py~sandybridge~sfad+unit_tests build_system=cmake build_type=Release generator=make sfadsize=4 arch=linux-sles15-zen3

so he is building Albany with compass.

@jewatkins
Copy link

jewatkins commented Mar 18, 2024

right, so I would just use those same settings in our nightly testing of albany spack. but feel free to add another build, I don't see any issue with that.

@xylar
Copy link
Collaborator Author

xylar commented Mar 18, 2024

@ikalash, the @compass-2024-03-13 tag is a specific tag of Albany for use in Compass. It doesn't build Compass or have anything else to do with Compass other than being the version for us to use.

You should not do your nightly tests with that, obviously, because you want to test the latest master or develop or whatever. You also should not include that whole build string above, just albany@develop+mpas~py~epetra+unit_tests if you want to mimic the Compass spack build because the others were determined by Spack, not requested by me.

@ikalash
Copy link

ikalash commented Mar 18, 2024

@ikalash, the @compass-2024-03-13 tag is a specific tag of Albany for use in Compass. It doesn't build Compass or have anything else to do with Compass other than being the version for us to use.

You should not do your nightly tests with that, obviously, because you want to test the latest master or develop or whatever. You also should not include that whole build string above, just albany@develop+mpas~py~epetra+unit_tests if you want to mimic the Compass spack build because the others were determined by Spack, not requested by me.

Thanks for clarifying @xylar , I think we are on the same page as I understood things the way you explained.

@mperego
Copy link

mperego commented Mar 18, 2024

@ikalash it might be good to have two spack builds for testing, one with the minimal options used in the compass build and then the one you have now. Maybe we can reduce the frequency of the nightly builds so that we do not use too many resources.

I think we should get rid of the Epetra option, since it's ignored by Albany anyway now.

@ikalash
Copy link

ikalash commented Mar 18, 2024

@ikalash it might be good to have two spack builds for testing, one with the minimal options used in the compass build and then the one you have now. Maybe we can reduce the frequency of the nightly builds so that we do not use too many resources.

Yes, this is what I was planning to do.

I think we should get rid of the Epetra option, since it's ignored by Albany anyway now.

I can do this.

@mperego
Copy link

mperego commented Mar 18, 2024

Great. Thanks.

@ikalash
Copy link

ikalash commented Mar 24, 2024

Just to follow up, I created the spack build we were discussing to run nightly on CEE and sends an email about the tests passing/not. @xylar let me know if you would like to be added to the list of people who get an automatic email about this daily.

@xylar
Copy link
Collaborator Author

xylar commented Mar 24, 2024

@ikalash, I appreciate the offer. I think at least for now I'd prefer not to be part of the daily emails but we can re-evalute later if it seems useful for me to follow.

@ikalash
Copy link

ikalash commented Mar 25, 2024

@xylar I understand. I will not spam you with the emails :).

@mperego
Copy link

mperego commented Mar 25, 2024

Thank you @ikalash for creating the spack build!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants